-
Notifications
You must be signed in to change notification settings - Fork 491
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
interactive-tx: Add dual-funding flow, using the interactive tx protocol (feature 28/29) #851
Conversation
26/27 already taken by #814 (at least, I proposed it move there to avoid clash)? Try 28/29? |
afac2ff
to
4c2d788
Compare
4c2d788
to
65f14c9
Compare
1603154
to
f53ca23
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ongoing review, awesome work!
Would it make sense to add optional Can be done by node ID too but this may lead to less UX hassle as the ID doesn't need to be transferred back-and-forth. |
`open_channel2` and `accept_channel2` are easily extensible via TLVs for
authentication schemes.
…On Thu, Apr 8, 2021 at 03:47 Martin Habovštiak ***@***.***> wrote:
Would it make sense to add optional auth_token field (up to 33 bytes?) so
that one could only allow dual-funded channels for authorized peers? Or
maybe to identify how much the other side wants to put into the channel.
Can be done by node ID too but this may lead to less UX hassle as the ID
doesn't need to be transferred back-and-forth.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#851 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAIMAKOVGO6NB7VE4DOZXQLTHVUQLANCNFSM4YN3DDXA>
.
|
Yes, I mean perhaps it'd be beneficial to have a standardized field so that clients implement the interface of filling that in. Would be real shame if only LND could open authenticated channel to LND, Elair to Ecliar and c-lightning to c-lightning... Basically have a way for the server to provide hex/base64 string that user fills in to get double-funded channel. This will be easier if all impls agree on the field being used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left few more comments, 3 high-level remarks in mind :)
-
Should we recommend to verify finalized interactive transaction against client's full-node transaction standardness verifier (i.e if you're using Core's
testmempoolaccept
) ? Even if we precise most of the well-known policy checks, we still have edge cases like interactive transaction's parent being a non-mature coinbase transaction. Likely impossible to handle for mobile clients, without assuming full-validation, for them detection of wrongdoing might be only a best-effort. -
Should we enrich failure modes ? Right now the only one detailed is broadcasting the commitment transaction thus stopping the cooperation with the counterparty. This might not work in adversarial settings, where your counterparty is pinning the interactive transaction to drive crazy your fee-bumping logic or DoS your committed coins or whatever. In that case of scenario, honest double-spend of one's inputs after a timeout might be wiser.
-
Should we introduce the interactive transaction section in its own BOLT ? As the 2 above points underscore it, multi-party funded transaction in the context of trust-minimized counterparty is far harder to get right than single-funded transaction. We migh land first this chunk, and from then enrich/harden the spec with best practices as we discover more edge cases and base layer support improves and we might add poodle or an equivalent solution to the privacy issues. Further, I think this interaction transaction protocol is best thing we have in the ecosystem which look like the specification of a coinjoin ? I can definitively see non-ln projects implementing it or seeking compatibility or extending it to achieve fancier stuff like cut-through.
02-peer-protocol.md
Outdated
added by the sending node | ||
- the `txid` does not match the txid of the transaction | ||
- MUST fail the channel if: | ||
- the `witness_stack` weight lowers the effective `feerate` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, this one is quite easy to bypass by an adversarial counterparty. Let's announce first a witnessScript with alternative script branches though diverging witnesses's weights. Branch A weight is 60 wu, branch B weight is 120 wu. You provide witness A to your honest counterparty, it satisfies the negotiated transaction feerate so honest signatures/witnesses are answered back. Now, you broadcast transaction with witness B, thus lowering transaction feerate.
Ideally, I think you could sanitize counterparty's witness through miniscript to learn if it's the most economical spending.
A `fee_step` 2 would be `1.25^2 * 512` (or 640 + 640 / 4), 800 sat/kw. | ||
|
||
If a valid `funding_locked` message is received in the middle of an | ||
RBF attempt, the attempt MUST be abandoned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we recommend to enforce bip125 rules 3 and 4 ? It's quite easy to increase the feerate by diminishing the number of inputs/outputs though paying less in absolute fee and thus hitting a rejection of the replacement ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally yes, but in practice it's impossible to pre-commit to a feerate that is guaranteed to surpass the last tx's absolute fee, given that the input set can be reconfigured.
Open to suggestions on how to enforce this however.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As currently implemented, in some cases you have to repeatedly RBF (and increment the feerate) until you hit upon a tx that works -- really not ideal but it works.
02-peer-protocol.md
Outdated
|
||
It's recommended that a peer, rather than fail the RBF negotiation due to | ||
a large feerate change, instead sets their `funding_satoshis` to zero, | ||
and decline to participate further in the channel funding. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have ambiguity there ? You might be interested to participate further in the interactive transaction confirmation but not in this specific channel as all its value is swallowed by the bumping feerate ?
Should init_rbf
/ack_rbf
be transaction-level messages and not channel-level ones ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should init_rbf/ack_rbf be transaction-level messages and not channel-level ones ?
I'm not sure I follow what you mean by this -- as described they re-initiate the tx-construction phase of the protocol.
I think this proposal is a great opportunity to provide a standard way of paying for liquidity (to help with the initial inbound liquidity problem). Maybe it's worth a separate PR that will enrich the The high-level idea is to simply introduce new TLVs to allow participants to:
Deducing the fee from the initial main outputs would then be deterministic and could be verified before finalizing the funding tx, which is great. It would remove the hackish uses of It would also provide an incentive for nodes to add funds to channels where they're not the initiator. Because without such a fee, it will be hard to automate the addition of funds when we're fundee, because we most likely don't want to add some of our funds to every channel that remote nodes open to us (otherwise they could easily deplete our utxo pool). Any thought on this high-level proposal before I start diving into technical details? |
I think this is a great idea! So great in fact that I've already written up a spec draft and have started working on implementing it. I just pushed up the draft version #878, and would love to collaborate, get your feedback on it. |
Hm. Ideally we would specify all of the "standardness" rules here so as not to be dependent on the 'standardness verifier' implementation, however that list seems to fairly changeable. Better to specify what to do in the case that the broadcast fails? (double spend the inputs/attempt another RBF).
Yeah, the 'double spending of an input to cancel an open' is a bit implicit here, it's probably worthwhile to call it out explicitly.
Yeah, I think this is a good idea, and something that we can do naturally/easily after this is merged (and more sections of the spec adopt it/use it) |
Most recent push is whitespace only changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 024df99
During yesterday's spec meeting, we discussed griefing attacks and pinning again, and suggested re-evaluating the option of making the opener send There are three pinning vectors that apply to interactive-tx transactions:
Nodes can avoid the first one by setting the The second one can be fixed by either constraining change outputs (to have a CSV 1 for example) or using v3 transactions when available. In practice though, as long as nodes ensure that the transaction has a reasonable feerate, it is very likely that it will confirm: it isn't that simple to reliably pin it. The third one can be fixed by either sending I'd like to highlight a few things to help guide the decision. First of all, if we keep the existing My personal opinion is that it's not worth introducing the @morehouse @niftynei what do you think? |
AFAICT, vector 2 only works in combination with vector 3. There's also another category of pinning, where mempools get partitioned -- a low-feerate transaction that conflicts with the funding tx can be pinned in some mempools while the funding tx is in the victim's mempool.
Perhaps we make this the default instead of a TLV?
I'm not sure these are magic bullets either. Either one can require the victim to spend some extra fees to CPFP or double-spend the funding tx. Under sustained griefing, the victim's funds could be slowly burned as fees.
Right, vector 3 is what enables the feerate to be reduced so that pinning can occur.
I think p2tr doesn't work, because even key-path spends can be inflated via the annex.
Sort of, but those funds don't need to actually be the attacker's. If Mallory targets 2 (or more) LSPs at once, she can use the LSPs' inputs against each other, ensuring she sends
Yeah, I'm not sure either. I made a Discourse post discussing liquidity griefing in depth. It would be great to get some discussion going over there. |
Right, that's a good point!
In the longer term, we should be able to get rid of this restriction, so I'd rather keep it opt-in. We discussed this a while ago, there are scenarios where it makes sense to use unconfirmed inputs to optimize liquidity usage (high fees and chains of unconfirmed txs), and if the other party is honest, this doesn't create any issue.
True, this can create a slow drain on the victim's funds if the attack is sustained. But that's only under the assumption that the transactions never confirm on their own, which they sometimes should if the feerate is reasonable, in which case it costs fees for the attacker, not the victim. If the feerate was too low and we're using v3, then a double-spend isn't necessarily costing anything to the victim, that double-spend would just pay the expected current feerate (and if this is double-spent in another liquidity ads attempt, it won't be the seller who pays the on-chain fees). |
How will we do this?
Good points. I did a little math to see what a typical griefing might cost an LSP. Some recent mempool feerates:
Let's say we use 80 sat/vB for a funding transaction and the LSP's portion of the transaction is 250 vB, so they allocate The attacker inflates their witnesses to reduce the feerate to 10 sat/vB and also chains a 1,000 vB descendant transaction (the max allowed by v3 policy) with a fee of 10,000 sat (10 sat/vB). This package is unlikely to confirm for a very long time. If the LSP wishes to double-spend their griefed UTXOs, they must now spend Suppose further that the attacker is doing repeated griefing and this new dual-funded transaction is actually with another node controlled by the attacker. The attacker can grief this transaction as well, causing the double-spend fee to increase to |
If an attacker lowers the effective feerate by using low-feerate ancestors, or double-spends one of the ancestor transactions, this is free to double-spend: a competing transaction can use the same feerate without any additional cost. The only cumbersome part is how long you wait before figuring out that something is fishy with that transaction and you should unlock its utxos. For that, you need a heuristic that "this tx seems to have a good enough feerate, but it hasn't confirmed for X blocks, so let's mark it as malicious". The attack becomes more painful when coupled with a descendant chain, and IMO the only way to fix this that doesn't have annoying drawbacks is to fix mempool policies ™️ 🤞 (v3 txs / cluster mempool). If we don't need to care about descendant pinning, this is thus not so much of an issue and confirmed inputs shouldn't be the default (as they create inefficient liquidity usage). I've detailed that point a bit more on your delving bitcoin post, maybe we should keep the discussion over there where it will be accessible to more readers than on this PR?
Nice work! But this pinning attack does not reliably work though, so I believe it's not the end of the world. To be able to effectively lower the feerate of the funding transaction, the attacker needs to have low-feerate ancestors. But since those ancestors are low-feerate, they will be dropped from a fraction of the network's mempools, which will allow an honest transaction at the normal feerate to go through (because in those mempools, that honest transaction isn't a double-spend, the conflicting transactions have been evicted). That's what @TheBlueMatt mentioned a few times, that in practice today, based on his experiments, even low-ish feerate transactions are still able to propagate and confirm. |
Make `require_confirmed_inputs` explicit for RBF regnegotiation. Requested-By: @t-bast
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 27ffef4, I think this is ready to be merged! 🎉
Let's discuss this during the next spec meeting (early january) and hopefully this will be our first merged PR in 2024!
Since this PR hasn't been merged yet, I'd like to get everyone's feedback on adding yet another change before integrating this: niftynei#18 After having hands-on experience doing support for users who are affected by this, I think it would be best to immediately provide the tools to get rid of this issue entirely now that taproot is a thing. The PR linked contains two design options to achieve that, let me know what you think is best. Even though we have running code using the current messages, I'm fully open to making backwards-incompatible changes to the spec if necessary to ensure a cleaner protocol in the future without the need for additional feature bits/negotiation. |
After figuring out the attack vectors in more details, I've retracted niftynei#18 (more details directly on that PR). So I'm stilli ACKing 27ffef4 and I think we should consider merging this PR! |
As discussed in yesterday's spec meeting, this PR has widespread ACKs so we can merge it whenever you want @niftynei 🎉 🎉 The only remaining open comment is this one: #851 (comment), up to you to decide what you want to do here (and we can also re-work the wording or rationale in a follow-up PR if necessary). |
Thanks @t-bast for shepherding this through for the last few months (years?). Would not have happened without his tireless focus on getting this done! Now we splice?? :) |
This commit adds the interactive transaction construction protocol, as
well as the first practical example of using it, v2 of channel
establishment.
Note that for v2 we also update the channel_id, which now uses the hash
of the revocation_basepoints. We move away from using the funding
transaction id, as the introduction of RBF makes it such that a single
channel may have many funding transaction id's over the course of
its lifetime.
The easiest way to understand the gist of this PR is this video, which explains the prior protocol and overviews this one.
https://youtu.be/i_GxmNZjwhk